Skip to content

Fix ICE in missing_const_for_fn #14776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 10, 2025
Merged

Fix ICE in missing_const_for_fn #14776

merged 1 commit into from
May 10, 2025

Conversation

samueltardieu
Copy link
Contributor

@samueltardieu samueltardieu commented May 10, 2025 •
edited by rustbot
Loading

The mir_drops_elaborated_and_const_checked query result has been stolen already and cannot be borrowed again. Use the optimized_mir query result instead.

changelog: [missing_const_for_fn]: fix ICE with some compilation options

Fixes #14774

r? @Jarcho

@samueltardieu
Fix ICE in missing_const_for_fn
9ed53b8 
The `mir_drops_elaborated_and_const_checked` query result has been
stolen already and cannot be borrowed again. Use the `optimized_mir`
query result instead.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 10, 2025
@samueltardieu
Copy link
Contributor Author

@blyxyas I'm not setup to do perf checks right now, don't hesitate if you want to comment on this change.

@y21
Copy link
Member

y21 commented May 10, 2025

For what it's worth, this actually used to use optimized_mir and was changed in #14003, so this is effectively a revert of that PR. I do agree though (as I've commented on that PR) that using mir_drops_elaborated_and_const_checked is too fragile and would sooner or later run into random ICEs, because it relies on the assumption that nothing else in the compiler/clippy is using MIR queries before that lint runs (presumably -Zvalidate-mir breaks that assumption), and I agree we should probably should keep using optimized_mir until we have a better solution for #6080.

blyxyas
blyxyas approved these changes May 10, 2025
View reviewed changes
Copy link
Member

@blyxyas blyxyas left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happened. It really happened that #14003 caused an ICE. I did black magic to try and perform a top 800 lintcheck run on that PR and it was all for nothing. That PR got delayed like 2 months for nothing.

At least it was Matthias who found it and not a random user.

As soon as this is merged, it should probably be backported.

@samueltardieu
Copy link
Contributor Author

@y21 @blyxyas Feel free to merge it and beta-nominate it, I assigned Jarcho just because when we talked about MIR during the latest meeting he joked that he ended up with all the MIR related PRs.

I'm not sure it deserves to be backported to stable even if there is a 1.87.1, as this is a nursery lint, and seems to only trigger with -Zvalidate-mir so far.

@blyxyas blyxyas added the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 10, 2025
@blyxyas blyxyas added this pull request to the merge queue May 10, 2025
Merged via the queue into rust-lang:master with commit 7f6d507 May 10, 2025
11 of 13 checks passed
@samueltardieu
Copy link
Contributor Author

In fact the ICE hadn't entered 1.87 since #14003 was merged some days after the last Clippy sync preceding the 1.87 beta branch creation. Backporting to beta will be enough indeed.

@samueltardieu samueltardieu deleted the issue-14774 branch May 12, 2025 08:43
@flip1995 flip1995 added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jun 19, 2025
@flip1995 flip1995 mentioned this pull request Jun 19, 2025
Open
@flip1995
Copy link
Member

rust-lang/rust#142725

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blyxyas blyxyas blyxyas approved these changes

Assignees

@Jarcho Jarcho

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ICE: attempted to read from stolen value
6 participants
@samueltardieu @y21 @flip1995 @blyxyas @Jarcho @rustbot